fix: Sub::Name B::GV introspection + stash aliasing for *Dst:: = *Src::#541
Merged
fix: Sub::Name B::GV introspection + stash aliasing for *Dst:: = *Src::#541
Conversation
B::svref_2object($cv)->GV->NAME previously returned "__ANON__" for CVs
renamed via Sub::Name::subname or Sub::Util::set_subname, because B.pm
required `defined &{$fqn}` to trust the name. Real-Perl XS Sub::Name
updates the CV's CvGV/CvNAME_HEK directly and B reads those fields
without consulting any stash entry, so the name is honored even when
the sub was never installed as a glob.
The `defined &{$fqn}` check was originally added to handle stash
deletion/clearing (op/stash.t, uni/stash.t), so it cannot be removed
outright. Instead, track an explicit `explicitlyRenamed` flag on
RuntimeCode that is set by Sub::Name::subname and Sub::Util::set_subname,
and have B.pm trust the name whenever that flag is set.
- RuntimeCode: add `explicitlyRenamed` field
- SubName.java / SubUtil.java: set the flag when renaming
- SubName: expose private helper `Sub::Name::_is_renamed($cv)`
- B.pm: consult the flag before falling back to the stash check
Impact on Sub-Name-0.28 t/exotic_names.t: 1038 more tests now pass
(0 -> 1038 pass out of 1560). Remaining 522 failures are unrelated to
Sub::Name and stem from stash-aliasing semantics
(`*palatable:: = *{"aliased::native::..."}`).
No regressions in op/stash.t / uni/stash.t (75/105 before and after).
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
4 tasks
299848a to
b195dee
Compare
87e54e7 to
5ccd1c3
Compare
Makes `*Dst:: = *Src::;` actually alias the namespaces at both the
storage and lookup levels, matching Perl 5 semantics — while preserving
the existing compile-time-qualified reference behaviour (`&Pkg::foo`
keeps pointing at its original CvGV, as real Perl does).
Before:
*Dst:: = *Src::;
\%Dst:: == \%Src:: # false (!!!)
sub Dst::foo {};
defined &Src::foo # false (!!!)
After:
*Dst:: = *Src::;
\%Dst:: == \%Src:: # true
eval 'sub Dst::foo {}';
defined &Src::foo # true
*{"Dst::sym"} = sub {};
defined &Src::sym # true
${"Dst::var"} = 42;
${"Src::var"} # 42
# transitive chains also collapse:
*Mid:: = *Src::;
*Fin:: = *Mid::;
\%Fin:: == \%Src:: # true
Implementation (see dev/prompts/stash-aliasing-plan.md for the design):
1. GlobalVariable infrastructure
- resolveAliasedFqn(fqn): resolves the package prefix of an FQN
through stashAliases. Fast path when the alias map is empty (no
hashing, no substring) — this is the overwhelmingly common case.
- resolvedStashAliasCache: memoised transitive resolution with a
16-hop cycle cap. Invalidated on setStashAlias/clearStashAlias/
resetAllGlobals. Non-aliased packages map to their own String
instance so callers can shortcut the substring+concat via
reference equality (`resolved == pkg`).
2. Hash storage unification in RuntimeGlob.set(RuntimeGlob)
When both glob names end with "::", also put the source's
RuntimeStash into globalHashes under the destination key. This
makes `\%Dst:: == \%Src::` true and `*Dst::{HASH} == *Src::{HASH}`
true.
3. Resolve-with-fallback on read-side accessors
getGlobalVariable / getGlobalArray / getGlobalHash / getGlobalCodeRef
and their `exists*` siblings first try the alias-resolved FQN; if
no value lives there, they fall back to the raw key. This preserves
compile-time-qualified references like `\&MCTest::Base::foo` that
point at CVs still living at their original FQN, matching real Perl
(which keeps each CV's CvGV pinned to the package it was compiled
in — the alias only redirects new writes and runtime symbolic refs).
4. Resolve-on-install for sub declarations
defineGlobalCodeRef always resolves through the alias so that
`*Dst:: = *Src::; sub Dst::foo {}` installs the sub under Src::foo.
SubroutineParser applies resolveAliasedFqn to the computed fullName
before splitting into code.packageName / code.subName, so caller()
and set_subname report the correct target package.
5. SubroutineParser package/name split bugfix
placeholder.subName was being set from the raw subName parameter,
which may contain "::" (parsing `sub Dst::foo {}` arrives with
subName="Dst::foo"). Combined with alias resolution rewriting the
full name to "Src::foo", this produced code.subName="Dst::foo"
and code.packageName="Src" and caller() reported "Src::Dst::foo".
Derive both halves from the resolved fullName instead.
Note: resolveAliasedFqn is *not* invoked inside NameNormalizer. That
route was too aggressive — it rewrote compile-time-qualified reads
like `\&MCTest::Base::foo`, causing a regression in
perl5_t/t/mro/method_caching.t where the sub still exists at its
original FQN after aliasing. The resolve-on-install + resolve-then-
fallback-on-read split avoids this.
Known limitation
----------------
GV-level aliasing — where a stash hash key stores a GV whose own name
differs from the hash key, e.g.:
${"palatable::"}{"sub"} = ${"palatable::"}{$encoded_sub};
is not supported. PerlOnJava's flat-map storage can't represent
"hash key X points to GV named Y" because globs are currently just
FQN strings without a separate name field. Tracked as follow-up in
the plan doc; it's what's blocking the remaining 392 failures in
Sub-Name-0.28 t/exotic_names.t.
Testing
-------
- New src/test/resources/unit/stash_aliasing.t with 5 subtests
(hash identity, sub installation, caller reporting, symbolic refs,
chained aliases) — all pass.
- make: all unit tests pass.
- perl5_t/t/op/stash.t + perl5_t/t/uni/stash.t: 75/105, unchanged.
- perl5_t/t/mro/method_caching.t: 31/36, unchanged from master.
- perl5_t/t/mro/method_caching_utf8.t: 28/28, unchanged from master.
- Sub-Name-0.28 t/exotic_names.t: +130 tests (1038 -> 1168 / 1560),
cumulative with the B/Sub::Name fix in the preceding commit.
Design doc: dev/prompts/stash-aliasing-plan.md (new)
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
5ccd1c3 to
b2d22f6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related fixes to PerlOnJava's symbol-table machinery, discovered via
jcpan -t Sub::Name:B::svref_2object($cv)->GV->NAMEnow honors names assigned bySub::Name::subname/Sub::Util::set_subname, matching real-Perl XS behaviour (the name is reported even when the sub was never installed as a glob).*Dst:: = *Src::now actually aliases the namespaces — both the stash-view hash storage and subsequent symbol lookups — matching Perl 5 semantics, while preserving the existing compile-time-qualified reference behaviour.Two commits, both based directly on
master.Commit 1 —
fix(B,Sub::Name): honor Sub::Name-assigned CV names in B::GV->NAMEB.pm'sB::CV::_introspectpreviously requireddefined &{$fqn}to trust the name reported bySub::Util::subname($cv). Real-Perl XSSub::Nameupdates the CV'sCvGV/CvNAME_HEKdirectly andBreads those fields without consulting any stash entry, so the name is honored even when the sub was never installed as a glob.The
defined &{$fqn}check can't simply be removed — it exists to handle stash deletion/clearing behaviour exercised byop/stash.t/uni/stash.t. Instead:RuntimeCode.explicitlyRenamedboolean.Sub::Name::subnameandSub::Util::set_subname.Sub::Name::_is_renamed($cv).B.pmtrusts the name when the flag is set, falling back to the stash check otherwise.Commit 2 —
feat(stash): alias storage + name resolution for *Dst:: = *Src::Five pieces, per
dev/prompts/stash-aliasing-plan.md:GlobalVariableinfrastructure. NewresolveAliasedFqn(fqn)plus memoised transitive-resolution cacheresolvedStashAliasCachewith cycle cap. Fast path whenstashAliases.isEmpty(); non-alias hits return the input string identity so callers can fast-path with==. Cache invalidated on every mutation andresetAllGlobals.Hash storage unification in
RuntimeGlob.set(RuntimeGlob)— when both names end in::, alsoglobalHashes.put(this.globName, getGlobalHash(value.globName)).Resolve-with-fallback on read-side accessors.
getGlobalVariable/getGlobalArray/getGlobalHash/getGlobalCodeRefand theirexists*siblings first try the alias-resolved FQN; if no value lives there, they fall back to the raw key. This preserves compile-time-qualified references like\&MCTest::Base::foothat point at CVs still living at their original FQN — real Perl keeps each CV'sCvGVpinned to the package it was compiled in, so the alias only redirects new writes and runtime symbolic refs.Resolve-on-install for sub declarations.
defineGlobalCodeRefalways resolves through the alias so*Dst:: = *Src::; sub Dst::foo {}installs inSrc::foo.SubroutineParseralso applies the resolver to the computedfullNamebefore splitting intocode.packageName/code.subName, socaller()andset_subnamereport the correct target package.SubroutineParserpackage/name split bugfix.placeholder.subNamewas being set from the raw parser token (could contain::). Combined with alias resolution rewriting the full name, this produced double-prefixedcaller()output (Src::Dst::foo). Both halves are now derived from the resolvedfullName.Caching strategy
resolveAliasedFqnis on the hot path for every global symbol access:if (stashAliases.isEmpty()) return fqn;— O(1) for programs that never use stash aliasing."Pkg::"maps inresolvedStashAliasCacheto its terminal target. Chains are collapsed once and reused until a mutation clears the cache. Non-aliased packages map to their own String instance so the caller uses reference equality to shortcut the substring+concat.Known limitation
GV-level aliasing — where a stash hash key stores a GV whose own name differs from the hash key:
${"palatable::"}{"sub"} = ${"palatable::"}{$encoded_sub};…is not yet supported. PerlOnJava's flat-map storage can't represent "hash key X points to GV named Y" because globs are currently just FQN strings without a separate
namefield. This is what's blocking the remainingexotic_names.tfailures; tracked as follow-up work in the plan doc.Test Impact
t/exotic_names.tperl5_t/t/op/stash.tperl5_t/t/uni/stash.tperl5_t/t/mro/method_caching.tperl5_t/t/mro/method_caching_utf8.tperl5_t/t/io/data.tunit/stash_aliasing.tmakeunit suiteperl5_t/t/suite (PR 519 baseline → this PR)Earlier revisions of this PR briefly regressed
mro/method_caching.t/mro/method_caching_utf8.t(by routingNameNormalizerthrough the alias resolver) andio/data.t(by breaking the DATA filehandle placeholder after*foo:: = *bar::in a BEGIN block). Both are fixed in the current revision:NameNormalizeris no longer alias-aware; install-site and read-site resolution happen inGlobalVariableaccessors with fallback.Dst::toSrc::at alias-assignment time (scoped to IO only — scalars/arrays/hashes/code keep their original FQN binding, matching real Perl).existsGlobalIOnow followsresolveStashHashRedirectso bareword-handle probes in an aliased package find the migrated placeholder.Remaining drops in the full-suite log are flaky timing failures under 18-job parallel load (e.g.
re/pat.t,re/speed.t,lib/croak.t,op/stat.t): they pass at master-baseline levels when run in isolation.Test plan
make— all unit tests pass./jperl src/test/resources/unit/stash_aliasing.t— 5/5 subtestsperl dev/tools/perl_test_runner.pl perl5_t/t/op/stash.t perl5_t/t/uni/stash.t perl5_t/t/mro/method_caching.t perl5_t/t/mro/method_caching_utf8.t— all at baselinejcpan -t Sub::Name— exotic_names.t 0 → 1168 passingGenerated with Devin